verify: upstream #64190 on its true base (tenki runners) - #948
verify: upstream #64190 on its true base (tenki runners)#948hashbender wants to merge 4 commits into
Conversation
Adds Tenki (tenki.cloud) as a seventh terminal execution backend alongside local, docker, ssh, singularity, modal, and daytona. Hermes creates Tenki sandboxes on demand for terminal, file tools, and execute_code, and terminates them on cleanup by default (opt-in pause/resume persistence via container_persistent: true). Core: - tools/environments/tenki.py: TenkiEnvironment — sandbox lifecycle, exec, pause/resume persistence, remote file sync-back - tools/tenki_config.py: profile-scope-aware auth/workspace/project/endpoint resolution from the Tenki CLI config or environment - Shared _container_config_from_env_config() helper replaces the three duplicated container-config dicts (terminal, file tools, execute_code) - Setup wizard, doctor, status, gateway, and CLI wiring; website docs, env-var reference, and cli-config.yaml.example - Optional tenki extra (tenki-sandbox==0.1.1), lazy-installed like modal/daytona Security & correctness hardening: - Do not inject the supervisor's control-plane Tenki token into the model-controlled guest env; host-side SDK auth is unchanged. Nested-sandbox creation is an explicit opt-in via terminal.tenki_forward_env (which also forwards the resolved token so `tenki login` credentials work), and logs a warning when the control-plane token is forwarded. - Resolve Tenki credentials and forwarded env through agent.secret_scope so an active profile scope wins over process-global os.environ and the shared machine CLI login is skipped when a profile scope is authoritative. - Strip TENKI_AUTH_TOKEN / TENKI_API_KEY from spawned subprocess environments (provider blocklist + always-strip tier), matching modal/daytona. - Namespace persistent sandbox identity by a per-profile token (name + metadata + reuse match) and resolve the snapshot-store path per profile, bound at construction so background-thread cleanup writes to the right home. - Durability gate: a non-durable snapshot is not recorded (cleanup pauses and preserves prior state); a failed pause leaves the sandbox live rather than terminating it. Restore falls back to a base image only for an unrecoverable snapshot (gone / non-durable / snapshot-specific invalid state), preserving the pointer on transient errors. - Config: blank tenki_api_endpoint default across both config loaders so the documented env/CLI fallback is reachable; allow the guest-home subtree (/home/tenki/*) as a valid cwd at all container-cwd guards. Known follow-up (pre-existing, backend-agnostic): the process-global terminal environment cache (_active_environments, keyed "default") is not profile-scoped, so under the multiplexing gateway forwarded credentials are not isolated across profiles. Tracked separately; documented in the credential-forwarding notes. Tests: tests/tools/test_tenki_environment.py plus terminal/file/config/scrub coverage, including profile-scope, durability, restore-classification, and cwd-subtree regression pins. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Required by contributor-check for nick@luxor.tech commits in NousResearch#64190. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@tenki-reviewer review this PR — full review please. This is the true-base build of upstream NousResearch#64190 (Tenki cloud sandbox terminal backend); the diff here is exactly what's proposed upstream. |
|
Review Complete Files Reviewed: 63 By Severity:
This PR adds the Tenki cloud sandbox backend across the Hermes codebase with new config keys, environment adapter, and tool integration. Review found 4 issues, including two high-severity: a stuck cleanup flag that permanently bricks the sandbox environment, and cross-profile credential reuse under the multiplexing gateway that can leak Tenki API credentials between profiles. Files Reviewed (63 files) |
![tenki-reviewer[bot]](https://avatars.githubusercontent.com/in/2900641?s=60&v=4){kind=small}
There was a problem hiding this comment.
Risk: 🟠 High (72/100) — 1 high finding, 1 low · 3881 LOC across 63 files
Overview
PR #948 introduces the Tenki cloud sandbox backend, adding a new environment adapter (tools/environments/tenki.py), 12 new config keys under env.tools.backends.tenki, env-var bridge wiring across CLI/gateway/tools, and security controls for credential isolation and env-blocklisting.
High-Severity Findings
Cross-profile credential reuse (finding-004)
The TenkiEnvironment bakes host-side credentials (_auth_token, _workspace_id, _project_id) into instance attributes at construction time. The shared environment cache _active_environments in tools/terminal_tool.py keys environments by effective_task_id, which collapses to 'default' for all profiles without isolation-keyed overrides. Under the multiplexing gateway, a second profile reuses the first profile's cached TenkiEnvironment — creating sandboxes, executing commands, and incurring billing against the wrong account.
Stuck cleanup flag bricks environment (finding-002)
TenkiEnvironment.cleanup() sets _cleanup_in_progress = True before calling _close_client(), which calls client.close() without exception handling. If close() raises (e.g., HTTP connection error), the exception propagates past the _cleanup_in_progress = False reset — permanently blocking all future sandbox operations until process restart.
Medium/Low-Severity Findings
- missing 'tenki' in
_CONTAINER_PATH_BACKENDS_FALLBACK(tools/file_tools.py:167): The fallback frozenset omits'tenki'while the primary constant_CONTAINER_BACKENDSincludes it. On import-failure code paths, file operations for tenki backends use host-path semantics instead of container-path semantics. - prompt_builder probe missing two tenki keys (
agent/prompt_builder.py:1008): The inlinecontainer_configdict omitstenki_sync_hermes_homeandtenki_forward_envthat the canonical shared builder includes, creating a maintenance drift hazard.
Assessment
Two high-severity issues warrant changes before merge: credential isolation under multiplexing and cleanup exception safety. The remaining two findings are lower risk but represent real consistency gaps in the tenki backend integration.
| def _close_client(client: Any) -> None: | ||
| if client is None: | ||
| return | ||
| close = getattr(client, "close", None) | ||
| if callable(close): | ||
| close() |
There was a problem hiding this comment.
🟠 Unhandled exception in _close_client permanently bricks TenkiEnvironment via stuck _cleanup_in_progress (bug)
TenkiEnvironment.cleanup() sets _cleanup_in_progress = True at line 978, then calls _close_client(client) in the finally block (line 1038) and in the sandbox-is-None early-return branch (line 981). _close_client() (lines 1082-1087) calls client.close() without an exception handler. If close() raises, the exception escapes, skipping the _cleanup_in_progress = False reset (lines 985 and 1044). All subsequent _ensure_sandbox() calls raise RuntimeError, permanently bricking the Tenki environment until process restart.
💡 Suggestion: Wrap the close() call inside _close_client with a try/except that logs and swallows exceptions, so no exception propagates back to cleanup()'s finally block or the sandbox-is-None branch.
📋 Prompt for AI Agents
In tools/environments/tenki.py, modify the _close_client static method (lines 1082-1087) to wrap the close() call at line 1087 in a try/except block that catches all exceptions, logs them at debug level, and does not re-raise. This ensures _close_client never propagates exceptions, allowing cleanup() to always reset _cleanup_in_progress to False.
| "tenki_max_duration": config.get("tenki_max_duration", 3600), | ||
| "tenki_idle_timeout": config.get("tenki_idle_timeout", 0), | ||
| "tenki_pause_retention": config.get("tenki_pause_retention", 0), | ||
| } |
There was a problem hiding this comment.
🟢 Probe container_config in prompt_builder.py missing two tenki keys compared to shared builder (bug)
agent/prompt_builder.py:_probe_remote_backend() builds its own container_config dict inline (lines 984-1008) for the tenki backend, but omits tenki_sync_hermes_home and tenki_forward_env that the canonical shared builder _container_config_from_env_config at tools/terminal_tool.py:1440-1467 includes. All three runtime callers (terminal_tool, code_execution_tool, file_tools) use the shared builder; only the prompt-builder probe hand-rolls its own dict. Since _create_environment defaults these safely, there is no runtime bug today — but the two dicts have already drifted and present a maintenance hazard.
💡 Suggestion: Either refactor the probe to call _container_config_from_env_config(config) instead of building its own inline dict, or add the missing tenki_sync_hermes_home and tenki_forward_env keys so the two definitions stay in sync.
📋 Prompt for AI Agents
In agent/prompt_builder.py, inside _probe_remote_backend() at lines 984-1008, add the two missing keys just before the closing brace of the container_config dict (after line 1007):
"tenki_sync_hermes_home": config.get("tenki_sync_hermes_home", False),
"tenki_forward_env": config.get("tenki_forward_env", []),
Alternatively, refactor to import and call _container_config_from_env_config(config) from tools.terminal_tool to eliminate the duplication entirely.
…l race cancel() can null self._sandbox between _ensure_sandbox() and the dereference in _start_process/_exec_raw/_transfer_sandbox, turning a user interrupt into an AttributeError. _require_sandbox() captures the reference under the lock and raises a typed RuntimeError if the sandbox was torn down. Found by Tenki Code Reviewer on the mirrored PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two findings from Tenki Code Reviewer on the true-base verification PR: - _close_client: swallow close() exceptions. cleanup() resets _cleanup_in_progress only after closing the client, so an escaping network error during teardown left the flag stuck and every later _ensure_sandbox() failed with 'Tenki cleanup is in progress'. - prompt_builder probe: replace the fourth inline container-config copy with the shared _container_config_from_env_config() builder; the inline dict omitted tenki_sync_hermes_home, tenki_forward_env, and docker_network, so probe environments diverged from real ones. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
Verification build for upstream NousResearch#64190 (Tenki cloud sandbox terminal backend).
Unlike mirror PR #947 — whose file overlay landed on the 11-day-stale mirror base and broke on an unrelated import (
skill_matches_platform_listdidn't exist in the oldagent/skill_utils.py) — this PR cherry-picks the actual PR commits onto their actual upstream base commit (c44de99) with only the workflow runner labels tenkified. CI results here reflect the real upstream PR.Deliberately NOT marked as a mirror so the watcher keeps tracking #947.